Skip to content

fix(cloudstack): get domain name information from network manager leases#6829

Open
ani-sinha wants to merge 3 commits intocanonical:mainfrom
ani-sinha:parse_nm_leases
Open

fix(cloudstack): get domain name information from network manager leases#6829
ani-sinha wants to merge 3 commits intocanonical:mainfrom
ani-sinha:parse_nm_leases

Conversation

@ani-sinha
Copy link
Copy Markdown
Contributor

@ani-sinha ani-sinha commented Apr 7, 2026

This is a series of three independent commits.
The first commit adds helper functions in dhcp module to get dhcp lease information from network manager.
The second commit fixes a mock leak in existing cloudstack unit test code.
The third commit adds ability in cloud stack code trying to get domain name use network manager leases as well (in addition to trying networtkd and isa dhclient as it does today already).

This change has been tested by our (Red Hat) customer. After the change, we see logs of this type:

2026-04-04 15:19:54,813 - DataSourceCloudStack.py[DEBUG]: FQDN requested
2026-04-04 15:19:54,813 - DataSourceCloudStack.py[DEBUG]: Try obtaining domain name from networkd leases
2026-04-04 15:19:54,813 - DataSourceCloudStack.py[DEBUG]: Could not obtain FQDN from networkd leases. Falling back to ISC dhclient
2026-04-04 15:19:54,813 - DataSourceCloudStack.py[DEBUG]: Could not obtain FQDN from ISC dhclient leases. Falling back to Network Manager leases
2026-04-04 15:19:54,814 - subp.py[DEBUG]: Running command ['/usr/bin/nmcli', '-f', 'DHCP4', 'd', 'show'] with allowed return codes [0] (shell=False, capture=True)
2026-04-04 15:19:54,827 - performance.py[DEBUG]: Running ['/usr/bin/nmcli', '-f', 'DHCP4', 'd', 'show'] took 0.014 seconds
2026-04-04 15:19:54,828 - DataSourceCloudStack.py[DEBUG]: Obtained the following FQDN: foo.bar.baz.com
2026-04-04 15:19:54,828 - xxx[DEBUG]: XYZ hostname: foo.bar.baz.com

Proposed Commit Message

Commit 1:

feat(dhcp): Add network manager lease parsing capability

DHCP leases can be directly obtained from network manager through appropriate
command to the network manager cli. Add a couple of helper functions to get
the lease information from network manager.

In a subsequent patch, we will use the helper function from cloud stack
datasource to get the lease information from network manager.

Commit 2:

test: fix test_get_domainname_isc_dhclient mock leak

Without mocking out dhcp.IscDhclient.get_newest_lease_file_from_distro, the
functions on some platforms returns None. This means
get_key_from_latest_lease() bails out early without calling parse_leases()
to parse the lease file. Fix it.

Commit 3:

fix(cloudstack): get domain name information from network manager leases

Red Hat uses network manager as the supported dhcp client. If network manager
cli is available, we should try to get domain name information directly from
the network manager leases before asking distro specific dhcp client
(dhcpcd by default).

Merge type

  • [] Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@ani-sinha ani-sinha mentioned this pull request Apr 7, 2026
Closed
2 tasks
ani-sinha added a commit to ani-sinha/cloud-init that referenced this pull request Apr 7, 2026
@ani-sinha
feat(dhcp): Add network manager lease parsing capability (canonical#6829
458334e 
)

DHCP leases can be directly obtained from network manager through appropriate
command to the network manager cli. Add a couple of helper functions to get
the lease information from network manager.

In a subsequent patch, we will use the helper function from cloud stack
datasource to get the lease information from network manager.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
ani-sinha added a commit to ani-sinha/cloud-init that referenced this pull request Apr 7, 2026
@ani-sinha
test: fix test_get_domainname_isc_dhclient mock leak (canonical#6829)
adf50c1 
Without mocking out dhcp.IscDhclient.get_newest_lease_file_from_distro, the
functions on some platforms returns None. This means
get_key_from_latest_lease() bails out early without calling parse_leases()
to parse the lease file. Fix it.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
ani-sinha added a commit to ani-sinha/cloud-init that referenced this pull request Apr 7, 2026
@ani-sinha
fix(cloudstack): get domain name information from network manager lea…
0056499 
…ses (canonical#6829)

Red Hat uses network manager as the supported dhcp client. If network manager
cli is available, we should try to get domain name information directly from
the network manager leases before asking distro specific dhcp client
(dhcpcd by default).

Signed-off-by: Ani Sinha <anisinha@redhat.com>
@ani-sinha
Copy link
Copy Markdown
Contributor Author

ani-sinha commented Apr 8, 2026 •
edited
Loading

@holmanb @TheRealFalcon please review

blackboxsw
blackboxsw requested changes Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this proposal @ani-sinha. I think it is almost there, but needs a bit more support awareness of multi-NIC environments with two active interfacees and test coverage in test_dhcp.py.

Comment thread cloudinit/net/dhcp.py Outdated
Comment thread cloudinit/net/dhcp.py
Comment thread cloudinit/net/dhcp.py Outdated
Comment thread cloudinit/net/dhcp.py Outdated
Comment thread tests/unittests/sources/test_cloudstack.py Outdated
@blackboxsw blackboxsw requested a review from Copilot April 14, 2026 18:37
@blackboxsw blackboxsw self-assigned this Apr 14, 2026
Copilot AI reviewed Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends CloudStack hostname/FQDN discovery by adding NetworkManager (nmcli) DHCP lease parsing helpers and using them as an additional fallback when determining the DHCP-provided domain name. It also adjusts CloudStack unit tests to avoid a mock leak and to cover the new NetworkManager fallback behavior.

Changes:

  • Add nmcli-based DHCP4/DHCP6 lease parsing helpers in cloudinit/net/dhcp.py.
  • Update DataSourceCloudStack._get_domainname() to attempt NetworkManager leases after ISC dhclient leases.
  • Fix/extend unit tests to properly mock ISC dhclient lease file selection and to validate the NetworkManager path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
cloudinit/net/dhcp.py Adds helper functions to run nmcli and parse NetworkManager DHCP lease output.
cloudinit/sources/DataSourceCloudStack.py Adds a NetworkManager lease fallback when determining domain name for FQDN construction.
tests/unittests/sources/test_cloudstack.py Updates mocks to avoid platform-dependent behavior and adds a test for the NetworkManager fallback path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cloudinit/sources/DataSourceCloudStack.py
Comment thread cloudinit/net/dhcp.py
Comment thread tests/unittests/sources/test_cloudstack.py Outdated
ani-sinha and others added 3 commits April 17, 2026 14:03
@ani-sinha @blackboxsw
feat(dhcp): Add network manager lease parsing capability (canonical#6829
f853216 
)

DHCP leases can be directly obtained from network manager through appropriate
command to the network manager cli. Add a couple of helper functions to get
the lease information from network manager.

In a subsequent patch, we will use the helper function from cloud stack
datasource to get the lease information from network manager.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
Co-authored-by: Chad Smith <chad.smith@canonical.com>
@ani-sinha
test: fix test_get_domainname_isc_dhclient mock leak (canonical#6829)
7f01176 
Without mocking out dhcp.IscDhclient.get_newest_lease_file_from_distro, the
functions on some platforms returns None. This means
get_key_from_latest_lease() bails out early without calling parse_leases()
to parse the lease file. Fix it.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
@ani-sinha @blackboxsw
fix(cloudstack): get domain name information from network manager lea…
96a8a02 
…ses (canonical#6829)

Red Hat uses network manager as the supported dhcp client. If network manager
cli is available, we should try to get domain name information directly from
the network manager leases before asking distro specific dhcp client
(dhcpcd by default).

Signed-off-by: Ani Sinha <anisinha@redhat.com>
Co-authored-by: Chad Smith <chad.smith@canonical.com>
@ani-sinha ani-sinha requested a review from blackboxsw April 17, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@blackboxsw blackboxsw Awaiting requested review from blackboxsw

Requested changes must be addressed to merge this pull request.

Assignees

@blackboxsw blackboxsw

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ani-sinha @blackboxsw